-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inserter: Show messaging when no blocks found #4040
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me and things are working as expected.
@@ -255,6 +256,14 @@ export class InserterMenu extends Component { | |||
} | |||
|
|||
renderCategories( visibleBlocksByCategory ) { | |||
if ( isEmpty( visibleBlocksByCategory ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the only usage of this logic is when searching it may make sense to make this verification in line 358 where we call render categories for search as it makes things more explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that makes sense. It's a bit complicated with trying to render the result of this.getVisibleBlocksByCategory
inline, since we'd need to both test its non-emptiness and pass as the argument to renderCategories
(or call twice, which is wasteful).
Lodash's _.cond
could handle this pretty nicely, though maybe a bit less obvious to read?
cond( [
[ isEmpty, () => (
<span className="editor-inserter__no-results">
{ __( 'No blocks found' ) }
</span>
) ],
[ () => true, this.renderCategories ]
] )( this.getVisibleBlocksByCategory( this.getBlockTypes() ) );
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version with lodash cond is nice, but it is not obvious to read as you pointed out. The other possibility would save getVisibleBlocksByCategory in a const before the render but this way we may be computing it without need. I did not think about the complication because of being inline. It looks like our options have their downsides are not better than the current version we have, so I think we can ignore this detail and merge as it is right now.
This pull request seeks to add messaging when searching using the inserter reveals no available block options.
Testing instructions:
Verify that if no blocks are found by searching the inserter menu that a "No blocks found" message is shown instead.